🎨 Reduce the number of acquisition/release of DB connection inside function repository #7904
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sanderegg
added
the
a:webserver
sanderegg
requested review from
giancarloromeo and
matusdrobuliak66
as code owners
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the functions repository to consolidate connection handling, improve transactional usage, and leverage Pydantic’s new from_attributes support for model validation.
- Wraps all repository operations in
pass_or_acquire_connectionto reuse or acquire DB connections. - Updates
transaction_contextusage to work with the returnedtransactionobject for both streams and executes. - Removes manual
dict(row)conversions by enabling Pydantic’smodel_config = ConfigDict(from_attributes=True).
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| services/web/server/src/simcore_service_webserver/functions/_functions_repository.py | Added pass_or_acquire_connection, adjusted transactions to use transaction, and simplified model_validate calls. |
| packages/models-library/src/models_library/functions.py | Enabled Pydantic’s from_attributes parsing for DB row objects by adding model_config. |
Comments suppressed due to low confidence (2)
services/web/server/src/simcore_service_webserver/functions/_functions_repository.py:696
- [nitpick] The call to
check_user_permissionsuses a positionalconnargument, whereas other calls explicitly nameconnection=conn. For consistency and clarity, consider using the named parameterconnection=conn.
await check_user_permissions(
services/web/server/src/simcore_service_webserver/functions/_functions_repository.py:125
- [nitpick]
get_asyncpg_engine(app)is being called repeatedly in each function. Consider fetching the engine once (e.g.,engine = get_asyncpg_engine(app)) at the start of the repository module or function to reduce overhead.
async with pass_or_acquire_connection(get_asyncpg_engine(app), connection) as conn:
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7904 +/- ##
==========================================
+ Coverage 87.97% 89.68% +1.70%
==========================================
Files 1845 1454 -391
Lines 71153 60138 -11015
Branches 1220 476 -744
==========================================
- Hits 62594 53932 -8662
+ Misses 8207 6085 -2122
+ Partials 352 121 -231
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
pcrespov
approved these changes
Jun 16, 2025
Member
Author
|
@mergify queue |
Contributor
🟠 Waiting for conditions to match
|
giancarloromeo
approved these changes
Jun 16, 2025
bisgaard-itis
approved these changes
Jun 16, 2025
Contributor
bisgaard-itis
left a comment
bisgaard-itis
left a comment
There was a problem hiding this comment.
Cool! Thanks a lot for the quick fix 👍🏻
GitHK
approved these changes
Jun 17, 2025
sanderegg
force-pushed
the
i1913/use-one-connection
branch
from
c06a353 to
b7c90c8
Compare
matusdrobuliak66
approved these changes
Jun 17, 2025
Contributor
|
@sanderegg I believe this PR also closes Can I get you to set this in your PR description? (I didn't do it myself in case you disagree) |
wvangeit
approved these changes
Jun 17, 2025
Contributor
wvangeit
left a comment
wvangeit
left a comment
There was a problem hiding this comment.
Had a look again. Looks good to me as far as I can see.
sanderegg
force-pushed
the
i1913/use-one-connection
branch
from
0059151 to
f0db78d
Compare
sanderegg
force-pushed
the
i1913/use-one-connection
branch
from
043d7dd to
ceb430f
Compare
Member
Author
|
@mergify queue |
Contributor
✅ The pull request has been merged automaticallyThe pull request has been merged automatically at eccf0c0 |
|
17 tasks
14 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What do these changes do?
As spotted with @bisgaard-itis we discovered that running under load some functions would timeout while trying to acquire a DB connection. This should improve that situation.
Related issue/s
How to test
Dev-ops